Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix clearing up Door Lock users and credentials #37900

Closed
wants to merge 1 commit into from
Closed

Fix clearing up Door Lock users and credentials #37900

wants to merge 1 commit into from

Conversation

adigie
Copy link
Member

@adigie adigie commented Mar 6, 2025

Problem

Door Lock user/credential is not cleared during factory reset. The issue was observed on nRF52 and nRF53 platforms.

Reproduction steps:

  1. Commission device
  2. Set user
chip-tool doorlock set-user 0 1 xxx 6452 1 0 0 1 1  --timedInteractionTimeoutMs 1000
  1. Set credential
chip-tool doorlock set-credential 0 '{ "credentialType": 1, "credentialIndex": 1 }' "12345678" 1 null null 1 1 --timedInteractionTimeoutMs 10000
  1. Factory reset
  2. Commission device
  3. Get user
chip-tool doorlock get-user 1 1 1
Result
GetUserResponse: {
  userIndex: 1
  userName: xxx
  userUniqueID: 6452
  userStatus: 1
  userType: 0
  credentialRule: 0
  credentials: 1 entries
    [1]: {
      CredentialType: 1
      CredentialIndex: 1
     }
  creatorFabricIndex: 0
  lastModifiedFabricIndex: 0
  nextUserIndex: null
 }

Changes

Properly clear user/credential by setting status to Available and clearing other attributes when removing user/credential during OnFabricRemoved.

Testing

Manually verified removal of user/credential during factory reset.

@adigie
Fix clearing up Door Lock users and credentials
81aec8a 
Properly clear user/credential by setting status to Available and
clearing other attributes when removing user/credential during
`OnFabricRemoved`.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@github-actions github-actions bot added the app label Mar 6, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 6, 2025 •
edited
Loading

PR #37900: Size comparison from ee95bd2 to 81aec8a

Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section ee95bd2 81aec8a change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1096882 1096882 0 0.0
RAM 94842 94842 0 0.0
bl702 lighting-app bl702+eth FLASH 651856 651856 0 0.0
RAM 33509 33509 0 0.0
bl702+wifi FLASH 829128 829128 0 0.0
RAM 22233 22233 0 0.0
bl706+mfd+rpc+littlefs FLASH 1061524 1061524 0 0.0
RAM 32157 32157 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 892368 892368 0 0.0
RAM 26896 26896 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 975264 975264 0 0.0
RAM 24644 24644 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 817224 817224 0 0.0
RAM 120272 120272 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 826144 826192 48 0.0
RAM 125368 125368 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 773028 773028 0 0.0
RAM 113740 113740 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 757288 757288 0 0.0
RAM 113948 113948 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 540790 540790 0 0.0
RAM 205128 205128 0 0.0
lock CC3235SF_LAUNCHXL FLASH 574890 574954 64 0.0
RAM 205376 205376 0 0.0
cyw30739 light CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 658925 658925 0 0.0
RAM 75412 75412 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 678785 678785 0 0.0
RAM 78052 78052 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 678785 678785 0 0.0
RAM 78052 78052 0 0.0
CYW930739M2EVB-02 unknown 2040 2040 0 0.0
FLASH 635709 635709 0 0.0
RAM 70480 70480 0 0.0
light-switch CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 619165 619165 0 0.0
RAM 71652 71652 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 638801 638801 0 0.0
RAM 74196 74196 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 638801 638801 0 0.0
RAM 74196 74196 0 0.0
lock CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 638653 638709 56 0.0
RAM 74660 74660 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 658377 658433 56 0.0
RAM 77204 77204 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 658377 658433 56 0.0
RAM 77204 77204 0 0.0
thermostat CYW30739B2-P5-EVK-01 unknown 2040 2040 0 0.0
FLASH 614993 614993 0 0.0
RAM 68748 68748 0 0.0
CYW30739B2-P5-EVK-02 unknown 2040 2040 0 0.0
FLASH 634853 634853 0 0.0
RAM 71388 71388 0 0.0
CYW30739B2-P5-EVK-03 unknown 2040 2040 0 0.0
FLASH 634853 634853 0 0.0
RAM 71388 71388 0 0.0
efr32 lock-app BRD4187C FLASH 939760 939792 32 0.0
RAM 159920 159920 0 0.0
BRD4338a FLASH 733376 733456 80 0.0
RAM 234840 234840 0 0.0
window-app BRD4187C FLASH 1032264 1032264 0 0.0
RAM 128024 128024 0 0.0
esp32 all-clusters-app c3devkit DRAM 98704 98704 0 0.0
FLASH 1593362 1593362 0 0.0
IRAM 83820 83820 0 0.0
m5stack DRAM 117492 117492 0 0.0
FLASH 1560042 1560042 0 0.0
IRAM 117039 117039 0 0.0
linux air-purifier-app debug unknown 4752 4752 0 0.0
FLASH 2653835 2653835 0 0.0
RAM 112304 112304 0 0.0
all-clusters-app debug unknown 5560 5560 0 0.0
FLASH 5975316 5975316 0 0.0
RAM 515608 515608 0 0.0
all-clusters-minimal-app debug unknown 5456 5456 0 0.0
FLASH 5311924 5312198 274 0.0
RAM 222648 222648 0 0.0
bridge-app debug unknown 5472 5472 0 0.0
FLASH 4625648 4625648 0 0.0
RAM 200984 200984 0 0.0
camera-app debug unknown 5456 5456 0 0.0
FLASH 4675742 4675742 0 0.0
RAM 195792 195792 0 0.0
camera-controller debug unknown 5776 5776 0 0.0
FLASH 11279431 11279431 0 0.0
RAM 594048 594048 0 0.0
chip-tool debug unknown 6112 6112 0 0.0
FLASH 13292411 13292411 0 0.0
RAM 602944 602944 0 0.0
chip-tool-ipv6only arm64 unknown 21992 21992 0 0.0
FLASH 11488152 11488152 0 0.0
RAM 655536 655536 0 0.0
fabric-admin debug unknown 5800 5800 0 0.0
FLASH 11573059 11573059 0 0.0
RAM 602728 602728 0 0.0
fabric-bridge-app debug unknown 4720 4720 0 0.0
FLASH 4456604 4456604 0 0.0
RAM 188168 188168 0 0.0
fabric-sync debug unknown 4976 4976 0 0.0
FLASH 5573557 5573557 0 0.0
RAM 470864 470864 0 0.0
lighting-app debug+rpc+ui unknown 6192 6192 0 0.0
FLASH 5519953 5519953 0 0.0
RAM 205168 205168 0 0.0
lock-app debug unknown 5424 5424 0 0.0
FLASH 4692424 4692564 140 0.0
RAM 192344 192344 0 0.0
ota-provider-app debug unknown 4760 4760 0 0.0
FLASH 4314602 4314602 0 0.0
RAM 181000 181000 0 0.0
ota-requestor-app debug unknown 4712 4712 0 0.0
FLASH 4444922 4444922 0 0.0
RAM 185488 185488 0 0.0
shell debug unknown 4240 4240 0 0.0
FLASH 2952188 2952188 0 0.0
RAM 145424 145424 0 0.0
thermostat-no-ble arm64 unknown 9448 9448 0 0.0
FLASH 4141672 4141672 0 0.0
RAM 229808 229808 0 0.0
tv-app debug unknown 5752 5752 0 0.0
FLASH 5912405 5912405 0 0.0
RAM 594296 594296 0 0.0
tv-casting-app debug unknown 5320 5320 0 0.0
FLASH 11463053 11463053 0 0.0
RAM 718208 718208 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 914784 914784 0 0.0
RAM 142881 142881 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 907992 907992 0 0.0
RAM 125221 125221 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 851604 852176 572 0.1
RAM 141243 141243 0 0.0
nxp contact k32w0+release FLASH 587440 587440 0 0.0
RAM 70980 70980 0 0.0
mcxw71+release FLASH 601264 601264 0 0.0
RAM 63096 63096 0 0.0
light k32w0+release FLASH 613172 613172 0 0.0
RAM 70268 70268 0 0.0
k32w1+release FLASH 685888 685888 0 0.0
RAM 48584 48584 0 0.0
lock mcxw71+release FLASH 750104 750152 48 0.0
RAM 67500 67500 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1660284 1660284 0 0.0
RAM 212320 212320 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1564588 1564668 80 0.0
RAM 208536 208536 0 0.0
light cy8ckit_062s2_43012 FLASH 1441324 1441324 0 0.0
RAM 197296 197296 0 0.0
lock cy8ckit_062s2_43012 FLASH 1470212 1470308 96 0.0
RAM 224960 224960 0 0.0
qpg lighting-app qpg6105+debug FLASH 663852 663852 0 0.0
RAM 105156 105156 0 0.0
lock-app qpg6105+debug FLASH 622312 622360 48 0.0
RAM 99768 99768 0 0.0
stm32 light STM32WB5MM-DK FLASH 459928 459928 0 0.0
RAM 141472 141472 0 0.0
telink bridge-app tl7218x FLASH 664564 664564 0 0.0
RAM 90712 90712 0 0.0
contact-sensor-app tlsr9528a_retention FLASH 622146 622146 0 0.0
RAM 31488 31488 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 760920 760920 0 0.0
RAM 40420 40420 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 754028 754028 0 0.0
RAM 97540 97540 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 681078 681078 0 0.0
RAM 52192 52192 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 709636 709636 0 0.0
RAM 73400 73400 0 0.0
light-switch-app-ota-shell-factory-data tl3218x_retention FLASH 702186 702186 0 0.0
RAM 37664 37664 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 601756 601756 0 0.0
RAM 138640 138640 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 789044 789044 0 0.0
RAM 96388 96388 0 0.0
tizen all-clusters-app arm unknown 5152 5152 0 0.0
FLASH 1780052 1780052 0 0.0
RAM 94152 94152 0 0.0
chip-tool-ubsan arm unknown 11500 11500 0 0.0
FLASH 18967150 18967150 0 0.0
RAM 8299328 8299328 0 0.0

@andy31415
Copy link
Contributor

@adigie could we automate an integration test to validate this? Do we have the ability to factory reset our test examples?

bzbarsky-apple
bzbarsky-apple requested changes Mar 6, 2025
View reviewed changes
src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -2173,6 +2173,16 @@ bool DoorLockServer::clearFabricFromUsers(chip::EndpointId endpointId, chip::Fab
user.lastModifiedBy = kUndefinedFabricIndex;
}

if (user.createdBy == kUndefinedFabricIndex && user.lastModifiedBy == kUndefinedFabricIndex)
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per spec, users/credentials should not be removed on fabric removal.

If they should be removed on factory reset (which is different from fabric removal), then that should happen in whatever code handles factory reset.

Of course if you think the spec here should change please bring that up in the relevant TT. But the door lock TT was very explicit about not removing users/credentials on fabric removal.

@adigie adigie marked this pull request as draft March 7, 2025 08:22
@adigie
Copy link
Member Author

adigie commented Mar 7, 2025

Closing as this will be handled in platform specific code that is not yet upstreamed.

@adigie adigie closed this Mar 7, 2025
@adigie adigie deleted the toup/fix-clear-doorlock-user-credential branch March 7, 2025 13:22
@adigie adigie mentioned this pull request Mar 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bzbarsky-apple bzbarsky-apple bzbarsky-apple requested changes

Assignees
No one assigned
Labels
app review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adigie @andy31415 @bzbarsky-apple